Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Token fixes and improvements #1618

Merged
merged 23 commits into from
Nov 27, 2023
Merged

feat!: Token fixes and improvements #1618

merged 23 commits into from
Nov 27, 2023

Conversation

espendk
Copy link
Collaborator

@espendk espendk commented Nov 24, 2023

The token name was misused: Sometimes it was assumed to be a symbol and sometimes an ID. This caused issues now that we use token IDs that are different from the symbols. (also, the name had nothing to do with the ERC20 name..)

Token name has therefore been replaced by id and symbol in all relevant places. Configuration files have been converted to use the token instance ID's from the context-addresses package to avoid ambiguity among (1) different tokens with the same symbol and (2) multiple token instances such as USDC (Circle issued) and USDC.e (bridged from Ethereum).

Default token IDs can be registered for a symbol and network. And if there is only one ID for a given symbol on a network, it will be considered the default. Mangrove.token() will create an instance of the default token ID if found.

In addition, the following changes have been made:

  • MgvToken has been renamed to Token. The prefix was at best confusing.
  • The Mangrove.openMarkets function now returns Tokens instead of a bespoke token data struct.
  • Mangrove.market and Market.connect now accept either symbol, token ID, or Token for base and quote.
  • {Mangrove, Token}.getTokenAddress(symbolOrId) function has been added.
  • Added displayName and displayedAsPriceDecimals to Token.
  • Token data is now always retrieved through the configuration and thus stored/cached in one place. A number of functions used to ignore the configuration and one even used a separate cache based on the moize package..
  • Throw error in Token.getDecimals if decimals are not on record.

Some unnecessary/redundant code/features have been removed as it simplified the task and made the code shorter and clearer:

  • Removed configuration.tokens.fetchDecimalsFromAddress. Instead, use Token.createTokenFromAddress and read the decimals from that token.
  • Removed Mangrove.getTokenAndAddress. Instead, use Mangrove.tokenFromAddress and read the address from there.
  • Removed Mangrove.tokenFromConfig. Use Mangrove.token instead.
  • Removed Mangrove.getNameFromAddress: It was ambiguous (multiple names could be registered) and only used for resolving tokens which can now be done with the new configuration.tokens.getTokenIdFromAddress() function.
  • Removed token configuration getters and setters from Mangrove and Token. Instead, use the methods on configuration.tokens.
  • Mangrove.toUnits|fromUnits no longer accepts a token name/symbol as this was ambiguous. Instead, use Token.createToken and call toUnits|fromUnits on that.

@espendk espendk force-pushed the feat/token-fixes branch 13 times, most recently from 867c163 to 0095d36 Compare November 24, 2023 14:37
Token `name` was misused: Sometimes it was assumed to be a symbol and sometimes an ID. It has therefore been replaced by `id` and `symbol` in all relevant places. Configuration files have been converted to use the token instance ID's from the context-addresses package to avoid ambiguity among (1) different tokens with the same symbol and (2) multiple token instances such as `USDC` (Circle issued) and `USDC.e` (bridged from Ethereum).
`Mangrove.getNameFromAddress` has been removed: It was ambiguous (multiple names could be registered) and only used for resolving tokens which can now be done with the new `configuration.tokens.getTokenIdFromAddress()` function.
@espendk espendk marked this pull request as ready for review November 25, 2023 00:08
@espendk espendk requested a review from lnist November 25, 2023 00:08
@espendk espendk changed the title Token fixes Token fixes and improvements Nov 25, 2023
src/configuration.ts Show resolved Hide resolved
src/configuration.ts Show resolved Hide resolved
src/configuration.ts Outdated Show resolved Hide resolved
src/configuration.ts Show resolved Hide resolved
src/kandel/kandelFarm.ts Outdated Show resolved Hide resolved
src/mangrove.ts Show resolved Hide resolved
src/mangrove.ts Show resolved Hide resolved
@espendk espendk requested a review from lnist November 27, 2023 12:12
@espendk espendk changed the title Token fixes and improvements feat!: Token fixes and improvements Nov 27, 2023
@espendk espendk enabled auto-merge (squash) November 27, 2023 12:56
@espendk espendk merged commit 7529b74 into develop Nov 27, 2023
3 checks passed
@espendk espendk deleted the feat/token-fixes branch November 27, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants